Skip to content

Conversation

@janine9vn
Copy link
Contributor

No description provided.

jb3 and others added 30 commits July 9, 2021 19:19
This project contains documentation and recommended tooling configuration for projects to be submitted during a Python Discord Code Jam.

Co-authored-by: Akarys42 <matteobertucci2004@gmail.com>
Co-authored-by: ChrisLovering <chris.lovering.95@gmail.com>
Co-authored-by: mbaruh <8bee278@gmail.com>
Co-authored-by: fisher60 <39353605+fisher60@users.noreply.github.com>
Changes the local pre-commit hooks out for remote ones, as well as
changes the action to use the pre-commit action, so pre-commit will no
longer depend on your project manager's settings.
Signed-off-by: GitHub <noreply@github.com>
Signed-off-by: GitHub <noreply@github.com>
Signed-off-by: GitHub <noreply@github.com>
Signed-off-by: GitHub <noreply@github.com>
Signed-off-by: GitHub <noreply@github.com>
- Use Python 3.10 instead of 3.9
- Update action versions to the newest major versions
- Remove caching configuration since it's not doing anything useful
  (and we would rather not have teams deal with caching issues)
Signed-off-by: GitHub <noreply@github.com>
*although I added '.0' at the end of flake8-docstrings's requirement since
it's present in the bot repository (where Kat got her versions anyway).
Update packaging and CI config for CJ9
Unfortunately installing isort from source errors out due to a poetry
bug(?). Since isort 5.x has a stability policy active, let's just bump
this so the template is usable (and upgrade the template properly
later).
The screenshot is my own as I couldn't find a GitHub provided one that works unfortunately. Admittedly ugly.
@MrGrote
Copy link

MrGrote commented Sep 20, 2025

README

The overall README reads well. I would however change up the order a bit, start with a short description of the project, followed by instructions on how to install and run the project, and lastly the details of the project. The addition of images to the README is nice, however the leading / prevents the images from being showed and the alt text of each image is the same while it only matches the first image. It could use some information about development like how to use the tools folder.

Commit Quality

Most commit titles are descriptive of what the was changed in the commit, however they do tend to be a bit terse. There is an absence of commit bodies throughout the whole commit history which could have been used to add context to why the commit was done. I noticed a few commits that also snuck in small changes in other files where they could have been their own commit.

Code Quality

The project was well structured and makes good use of classes. I appreciate the use of custom Exceptions. Type hinting was used in many places the application. It would be nice to add them everywhere unless the JavaScript api makes it impossible in those situations. There is some duplication in planets.json that I wonder could reduced and created in python. The overall code quality was good and easy to follow, even though docstrings were missing in some places. There were some cases of commented out code in the codebase, try not to commit this.

Summary

It was fun to fly around the universe scanning objects, it was impressive to see that this could be done with just python. I appreciated that the earth was surrounded by thrash.

One oversight I have noticed is that the game doesn't pause after you reach the victory state, making it so that you can reach the failure state at the same time. The project description was set a whimsical message "No idea yet :)" at the start of development but was never changed later on to match the project.


def render(self, ctx, timestamp):

# some temporary functionality for testing
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to be able to Toggle this testing mode with an env variable or a global

self.is_bobbing_up = not self.is_bobbing_up

draw_black_background(ctx)
#self.stars.render(ctx, timestamp)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#self.stars.render(ctx, timestamp)

Comment on lines +137 to +138
for star in replacement_stars:
self.stars.append(star)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for star in replacement_stars:
self.stars.append(star)
self.stars.extend(replacement_stars)

Comment on lines +15 to +16
@abstractmethod
def parse(self, text: str) -> object:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parse does not rely on the state of the object, it can be a staticmethod

data = response.read().decode()

if save_to:
with Path.open(save_to, "w") as f:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
with Path.open(save_to, "w") as f:
with save_to.open("w") as f:

Comment on lines +68 to +74
if self.highlighted:
if self.complete:
# log.debug("planet complete")
highlight = "#00ff00"
else:
# log.debug("planet not complete")
highlight = "#ffff00" # yellow highlight
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't leave commented out log statements behind in the code you commit.

if "d" in keys or "ArrowRight" in keys:
dx += 1.75

# TODO: remove this, for testing momentum
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have these work in a debug environment only instead of hardcoded

Comment on lines +258 to +286
# # Reduce scanner progress when hit by asteroid
# if hasattr(window, 'scanner') and window.scanner:
# # Reduce progress by 2-6% of max progress based on damage taken
# damage_taken = 100 / (distance_between_centers * 5 * asteroid.damage_mul
# progress_loss = window.scanner._bar_max * (0.02 + (damage_taken / 1000) * 0.04)
# window.scanner.scanning_progress = max(0, window.scanner.scanning_progress - progress_loss)
# # log.debug("Scanner progress reduced by %f due to asteroid collision", progress_loss)

window.audio_handler.play_bang()
window.debris.generate_debris(self.get_position(), Position(ast_x, ast_y))

def nudge_towards(self, pos: Position, gravity_strength: float = 0.75) -> None:
distance = self.get_position().distance(pos)
if distance == 0: return

x_dir = (pos.x - self.x) / distance
y_dir = (pos.y - self.y) / distance

self.x += x_dir * gravity_strength
self.y += y_dir * gravity_strength

# x_dir = math.cos((pos.x - self.x )/(pos.y - self.y))
# y_dir = math.sin((pos.x - self.x )/(pos.y - self.y))

# if abs(self.momentum[0]) < 1:
# self.momentum[0] += x_dir * momentum_amount
# if abs(self.momentum[1]) < 1:
# self.momentum[1] += y_dir * momentum_amount

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two large sections of commented out code




42 No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I finally understand

Comment on lines +69 to +70
col = self.sprite_index % self.grid_cols
row = self.sprite_index // self.grid_cols
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would have been a good opportunity to use the divmod builtin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.